Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes logging in with empty or null values #42

Merged
merged 2 commits into from
Apr 26, 2024
Merged

Conversation

A-Guldborg
Copy link
Contributor

Fixes #40

@A-Guldborg A-Guldborg requested a review from TTA777 April 23, 2024 19:27
@A-Guldborg A-Guldborg self-assigned this Apr 23, 2024
@A-Guldborg A-Guldborg added the bug Something isn't working label Apr 23, 2024
@A-Guldborg A-Guldborg marked this pull request as ready for review April 23, 2024 19:27
Copy link
Member

@TTA777 TTA777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would fix it yes, but let's rely on the built-in validation from the editForm instead. We are already using a viewModel with attributes for validation here, so we should be able to rely on that. I must have misconfigured something for the form when I created it, so that it fires the function before it validates if it is correct or not.

Let's investigate what is causing the bypassing of the existing validation, and fix that instead

@A-Guldborg A-Guldborg requested a review from TTA777 April 25, 2024 00:13
@A-Guldborg
Copy link
Contributor Author

A-Guldborg commented Apr 25, 2024

Ah, thanks for the hint @TTA777! The bypassing is caused by adding the login method call to the button's OnClick as well as on the OnValidSubmit.

The only downside to this solution is that the button is enabled despite the empty field, and only on attempted submit the field will show that the password cannot be empty.

Copy link
Member

@TTA777 TTA777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find, all good for me then. If the enabled button annoys you, you can look into the example. At least one of them has a disable button until the form is valid, though we may need to change the form type then. Also not something that bothers me

@A-Guldborg
Copy link
Contributor Author

Nice find, all good for me then. If the enabled button annoys you, you can look into the example. At least one of them has a disable button until the form is valid, though we may need to change the form type then. Also not something that bothers me

I just tried to see if it was possible, but I am not able to change that behaviour without introducing other unwanted behaviour.

What we can do, but gives other behaviour, is: Change the EditForm to use Context instead of Model. OnInitialized should then initialize the EditContext with an initialized LoginForm (instead of applying the LoginForm to the EditForm initially). This allows us to bind the Disabled property of the submit button to the EditContext.Validate().

The issue with this is: On load, the button will call the EditContext.Validate() and add a validation message to all invalid fields, so the page loads with:

The Email field is required.
The Password field is required.

The current version only validates each field individually on field changes and I couldn't figure out a way to combine this with the button disabled/enabled without adding much more code complexity. It might, as you say, require us to change away from the EditForm but I don't think that is worth it. I'll merge this but if our PO wants it in the future we have some idea of how to fix it / what our options are.

@A-Guldborg A-Guldborg merged commit 36ac002 into main Apr 26, 2024
2 checks passed
@A-Guldborg A-Guldborg deleted the fix/disallow-null-login branch April 26, 2024 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sign-in without typing in password gives unhandled error
2 participants